Pull Request Test Coverage Report for Build 9517068942

Details

• 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage remained the same at 96.916%

Totals
Change from base Build 9516791059:	0.0%
Covered Lines:	597
Relevant Lines:	616

💛 - Coveralls

Currently, I found a few warning for NXmpes

WARNING: A link was used for /entry/sample/bias_env/voltmeter, but no '@target' attribute was found.
WARNING: A link was used for /entry/sample/gas_pressure_env/pressure_gauge, but no '@target' attribute was found.
WARNING: A link was used for /entry/sample/temperature_env/temperature_sensor, but no '@target' attribute was found.

IMO, which should not be a warning because they are recommended fields. But it is not a problem if it does not affect our test.

One should write the target attribute when one uses a link, so it is reasonable to give a warning if it is not given in the file. Note that for the template validation, if @target is not present, we add it automatically.

For the tests that produce this warning, we should just regenerate the test files.

Also, some unit warning in xps example. Though units are there in the nexus file. Probably the reason you have not used the unit registry from pynxtools.

The unit /1_as_loaded__Survey/instrument/electronanalyzer/detector/raw_data/raw/@units = counts has no documentation.
The unit /1_as_loaded__Survey/instrument/electronanalyzer/detector/raw_data/cycle0_scan0/@units = counts has no documentation.

These are somewhat expected because NXdata/AXISNAME does not have any units attached. So the units actually are undocumented, so the log messages are expected.

Regarding SPM, I will skip the validation. The application definition will be restructured very soon.

I think in this particular case you do not need to change anything in pynxtools-spm, but it would be great if you could just regenerate the files once again. As you can see here, the tests only fail because we are now automaticall writing a @target attribute whenever links is used and therefore the logs are different. @RubelMozumder Would be great if you could simply update all SPM files accordingly, so that all tests pass here.

pynxtools-spm tests are still failing, but they work with FAIRmat-NFDI/pynxtools-spm#43

pynxtools-spm tests are still failing, but they work with FAIRmat-NFDI/pynxtools-spm#43

That PR was prepared for the branch hdf-base-validation. If you merge this PR to the master branch, I can merge that PR as well. Later, you can release a patch version for pynxtools.

Also, while playing around, I recognized some errors which are not recognized: This wrong dict:

  "/ENTRY/CALIBRATION[energy_referencing]":{
    "calibrated_axis": "@link:/entry/data",
    "calibrated_axis/@units": "eV",
    "physical_quantity": "energy",
    "reference_peak": "@attrs:metadata/scan_info/reference_energy",
    "binding_energy": 0.0,
    "binding_energy/@units": "eV"
  },

Generates the error ERROR: Expected a field at /ENTRY[entry]/CALIBRATION[energy_referencing]/calibrated_axis, but found a group. during validation, but the link is still being set, and this a units attribute is being written to /entry/data. This is not being picked up neither by the converter nor the validator. The wrong type of /entry/energy_referencing/calibrated_axis is neither reported by the validator.

Indeed, in #642 (specifically in commit 5941b1f), we decided that such keys shall not be removed, even if they are erroneous. I think this was the wrong decision then and we should remove such keys in order to still produce a valid file. I have implemented with 28371b0 and 372a879 now that such keys that are linked to the wrong NeXus type are removed. In order not to create nonsense in the file, we also need to remove any further subkeys (e.g., additional attributes for fields)

Here's the result for your example:

"/ENTRY/CALIBRATION[energy_referencing]":{
  "calibrated_axis": "@link:/entry/data",
  "calibrated_axis/@units": "eV",
  "physical_quantity": "energy",
  "reference_peak": "@attrs:metadata/scan_info/reference_energy",
  "binding_energy": 0.0,
  "binding_energy/@units": "eV"
},

ERROR: The type ('group') of '/ENTRY[entry]/CALIBRATION[energy_referencing]/calibrated_axis' conflicts with the concept /ENTRY/energy_referencing/calibrated_axis, which is of type 'field'.
ERROR: Expected a field at /ENTRY[entry]/CALIBRATION[energy_referencing]/calibrated_axis, but found a group.
WARNING: The group /ENTRY[entry]/CALIBRATION[energy_referencing]/calibrated_axis will not be written.
WARNING: The attribute /ENTRY[entry]/CALIBRATION[energy_referencing]/calibrated_axis/@units will not be written.
WARNING: The attribute /ENTRY[entry]/CALIBRATION[energy_referencing]/calibrated_axis/@target will not be written.

And for the other way around

"/ENTRY/CALIBRATION[energy_referencing]":"@link:/entry/data/energy",

ERROR: The type ('field') of '/ENTRY[entry]/CALIBRATION[energy_referencing]' conflicts with the concept /ENTRY/energy_referencing, which is of type 'group'.
ERROR: Expected a group at /ENTRY[entry]/CALIBRATION[energy_referencing], but found a field or attribute.
WARNING: The field /ENTRY[entry]/CALIBRATION[energy_referencing] will not be written.
WARNING: The attribute /ENTRY[entry]/CALIBRATION[energy_referencing]/@target will not be written.
ERROR: The type ('field') of '/ENTRY[entry]/CALIBRATION[energy_referencing]' conflicts with the concept /ENTRY/CALIBRATION, which is of type 'group'.

Note that in the latter case, we actually find two different conflicts (with the named group energy_referencing(NXcalibration) as well as with the unnamed (NXcalibration).

I think this should work nicely and also in the resulting files there's no confusion anymore.